30431 implement format for history with docs #30962

TheHipbot · 2017-02-13T03:15:42Z

- What I did
Adds the --format option to the history command. Part of #30431

- How I did it
Refactored history to use a formatter, add the format flag

- How to verify it
Wrote unit tests to check that formatter is working correctly.

- Description for the changelog

Adds --format option to history command

- A picture of a cute animal (not mandatory but encouraged)

AkihiroSuda · 2017-02-13T03:23:42Z

cli/command/formatter/history.go

+	trunc bool
+	h     image.HistoryResponseItem
+}
+


Please add MarshalJSON so that {{json .}} works

AkihiroSuda · 2017-02-13T03:24:05Z

cli/command/formatter/history_test.go

+	call       func() string
+}
+
+func TestHistoryContext_ID(t *testing.T) {


The granularity of the added tests seems different from existing tests.
(Please refer to existing *_test.go)

I added a test against the full table view, but I'm not sure if this is what you meant.

vdemeester

Design LGTM 🐮

TheHipbot · 2017-02-13T15:31:53Z

Sorry, I was having trouble running the integration tests, I'll fix this up tonight and update the PR.

TheHipbot · 2017-02-15T15:17:50Z

Is this still failing CI?

AkihiroSuda · 2017-02-20T08:22:10Z

I confirmed it works, so code LGTM

Please squash commits
Probably we don't need an integration test

TheHipbot · 2017-02-21T01:59:28Z

Squashed and removed the integration test.

AkihiroSuda · 2017-02-21T02:04:04Z

@yongtang PTAL?

yongtang · 2017-02-21T04:11:13Z

docs/reference/commandline/history.md

+| Placeholder | Description|
+| ---- | ---- |
+| `.ID` | Image ID |
+| `.CreatedSince` | Elapsed time since the image was created |


It seems both Created and CreatedSince has been defined for placeholders. Though here the Created is not mentioned in the docs.

I am wondering if we add both in the docs, or, maybe we could just merge Created and CreatedSince into one placeholder func?

yongtang · 2017-02-21T04:19:32Z

@TheHipbot it looks like --human is only useful when --format "table" (or no --format is used):

ubuntu@ubuntu:~/docker$ docker history --format '{{.Created}}' --human=false busybox
2017-01-13T14:13:54-08:00
2017-01-13T14:13:53-08:00
ubuntu@ubuntu:~/docker$ docker history --format 'table {{.Created}}' --human=false busybox
CREATED AT
2017-01-13T14:13:54-08:00
2017-01-13T14:13:53-08:00
ubuntu@ubuntu:~/docker$ docker history --format 'table {{.Created}}' --human busybox
CREATED AT
2017-01-13T14:13:54-08:00
2017-01-13T14:13:53-08:00
ubuntu@ubuntu:~/docker$ docker history --format 'table' --human busybox
IMAGE               CREATED             CREATED BY                                      SIZE                COMMENT
7968321274dc        5 weeks ago         /bin/sh -c #(nop)  CMD ["sh"]                   0B                  
<missing>           5 weeks ago         /bin/sh -c #(nop) ADD file:707e63805c0be1a...   1.11MB              
ubuntu@ubuntu:~/docker$

You may need to check the value of c.human inside Created() (and/or CreatedSince()) to render the output string in either human or non-human format accordingly. For example,

func (c *historyContext) Created() string {
	c.AddHeader(createdAtHeader)
        if c.human {
          .... 
        }
	return time.Unix(c.h.Created, 0).Format(time.RFC3339)
}

Also, I am wondering if it make sense to merge Created() and CreatedSince()?

TheHipbot · 2017-02-22T03:32:00Z

@yongtang I was originally thinking to provide both options, which I should have added to the docs. That being said it also makes sense to combine so I will update the PR with the two combined as Created() and update the docs to reflect that

tonistiigi · 2017-03-09T21:39:59Z

@yongtang PTAL

tonistiigi

LGTM

yongtang · 2017-03-09T22:52:25Z

docs/reference/commandline/history.md

+| `.CreatedAt` | Timestamp of when image was created |
+| `.CreatedBy` | Command that was used to create the image |
+| `.Size` | Image disk size |
+| `.Comment` | Comment for image |


Can you update the doc to align the table like below so that it is easy to update in the future?

| Placeholder | Description | | --------------- | ----------- | | `.ID` | Image ID | | `.CreatedSince` | Elapsed time since the image was created if --human=true, otherwise timestamp of when image was created | | `.CreatedAt` | Timestamp of when image was created | | `.CreatedBy` | Command that was used to create the image | | `.Size` | Image disk size | | `.Comment` | Comment for image |

yongtang · 2017-03-09T22:59:02Z

cli/command/formatter/history.go

+	var created string
+	created = units.HumanDuration(time.Now().UTC().Sub(time.Unix(int64(c.h.Created), 0)))
+	return created + " ago"
+}


Also, it seems that CreatedAt and CreatedSince is pretty much the same except for the ago. And --human flag does not have an impact to the rendering

I am wondering if CreatedAt should use time.Unix(int64(c.h.Created), 0).String() (just like docker images --format) instead?

I ended up having to separate these again due to some changes in how the headers are output with the formatters. I added to the docs to have both in the template.

…ormatter Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com> Adds to history documentation for --format Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com> Adds MarshalJSON to historyContext for {{json .}} format Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com> Adds back the --human option to history command Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com> Cleans up formatter around --human option for history, Adds integration test for --format option of history Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com> Adds test for history formatter checking full table results, Runs go fmt on touched files Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com> Fixes lint errors in formatter/history Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com> Runs go fmt on cli/command/formatter/history.go Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com> sRemoves integration test for --format option of history Merges Created and CreatedSince in docker history formatter, Updates docs and tests

TheHipbot · 2017-03-13T14:14:04Z

Looks like Jenkins ran out of space on one of those builds

vdemeester

LGTM 🐮
/cc @thaJeztah for docs review

tonistiigi · 2017-04-13T23:46:46Z

ping @thaJeztah

thaJeztah

LGTM

I agree with #30962 (comment), but it's just a nit so let's get this merged. I can do a quick follow up to address it

…or-history-with-docs 30431 implement format for history with docs

GordonTheTurtle added the status/0-triage label Feb 13, 2017

AkihiroSuda reviewed Feb 13, 2017

View reviewed changes

AkihiroSuda mentioned this pull request Feb 13, 2017

[UX] implementation status of the --format #30431

Closed

24 tasks

vdemeester added status/1-design-review status/failing-ci Indicates that the PR in its current state fails the test suite and removed status/0-triage labels Feb 13, 2017

vdemeester reviewed Feb 13, 2017

View reviewed changes

TheHipbot force-pushed the 30431-implement-format-for-history-with-docs branch 3 times, most recently from fb537b9 to 6b40c3c Compare February 14, 2017 04:48

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Feb 14, 2017

TheHipbot force-pushed the 30431-implement-format-for-history-with-docs branch from 2438070 to 9523ac5 Compare February 14, 2017 15:26

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Feb 14, 2017

cpuguy83 added status/2-code-review and removed status/1-design-review status/failing-ci Indicates that the PR in its current state fails the test suite labels Feb 17, 2017

TheHipbot force-pushed the 30431-implement-format-for-history-with-docs branch from 39ea636 to 3be6e78 Compare February 21, 2017 01:58

yongtang reviewed Feb 21, 2017

View reviewed changes

TheHipbot force-pushed the 30431-implement-format-for-history-with-docs branch from 3be6e78 to 49c9964 Compare February 22, 2017 03:49

GordonTheTurtle assigned tonistiigi Feb 27, 2017

TheHipbot force-pushed the 30431-implement-format-for-history-with-docs branch from 49c9964 to ad95a3b Compare March 4, 2017 18:11

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Mar 6, 2017

TheHipbot force-pushed the 30431-implement-format-for-history-with-docs branch from 39ff38d to 60cbecd Compare March 7, 2017 01:27

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Mar 7, 2017

tonistiigi added the rebuild/windowsRS1 label Mar 9, 2017

GordonTheTurtle removed the rebuild/windowsRS1 label Mar 9, 2017

tonistiigi approved these changes Mar 9, 2017

View reviewed changes

yongtang reviewed Mar 9, 2017

View reviewed changes

TheHipbot force-pushed the 30431-implement-format-for-history-with-docs branch from 60cbecd to 6bc1f34 Compare March 11, 2017 20:04

tonistiigi added the rebuild/powerpc label Mar 13, 2017

GordonTheTurtle removed the rebuild/powerpc label Mar 13, 2017

yongtang added the rebuild/powerpc label Mar 15, 2017

GordonTheTurtle removed the rebuild/powerpc label Mar 15, 2017

vdemeester approved these changes Mar 31, 2017

View reviewed changes

vdemeester added status/3-docs-review and removed status/2-code-review labels Mar 31, 2017

thaJeztah approved these changes Apr 16, 2017

View reviewed changes

thaJeztah merged commit a3ab463 into moby:master Apr 16, 2017

thaJeztah added this to the 17.06 milestone Apr 16, 2017

thaJeztah added impact/changelog impact/cli impact/documentation labels Apr 16, 2017

thaJeztah mentioned this pull request Apr 16, 2017

Minor fixups for history CLI reference #32645

Merged

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#30962 from TheHipbot/30431-implement-format-f…

1d73df5

…or-history-with-docs 30431 implement format for history with docs

AkihiroSuda mentioned this pull request Apr 25, 2017

Can docker image history return JSON showing image layers? docker/cli#4

Closed

albers mentioned this pull request Apr 27, 2017

Add bash completion for history --format #32883

Merged

thaJeztah mentioned this pull request Apr 2, 2018

Fix --format example for docker history docker/cli#980

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

30431 implement format for history with docs #30962

30431 implement format for history with docs #30962

TheHipbot commented Feb 13, 2017

AkihiroSuda Feb 13, 2017 •

edited

AkihiroSuda Feb 13, 2017

TheHipbot Feb 14, 2017

vdemeester left a comment

TheHipbot commented Feb 13, 2017

TheHipbot commented Feb 15, 2017

AkihiroSuda commented Feb 20, 2017

TheHipbot commented Feb 21, 2017

AkihiroSuda commented Feb 21, 2017

yongtang Feb 21, 2017

yongtang commented Feb 21, 2017 •

edited

TheHipbot commented Feb 22, 2017

tonistiigi commented Mar 9, 2017

tonistiigi left a comment

yongtang Mar 9, 2017

yongtang Mar 9, 2017

TheHipbot Mar 13, 2017

TheHipbot commented Mar 13, 2017

vdemeester left a comment

tonistiigi commented Apr 13, 2017

thaJeztah left a comment

30431 implement format for history with docs #30962

30431 implement format for history with docs #30962

Conversation

TheHipbot commented Feb 13, 2017

AkihiroSuda Feb 13, 2017 • edited

Choose a reason for hiding this comment

AkihiroSuda Feb 13, 2017

Choose a reason for hiding this comment

TheHipbot Feb 14, 2017

Choose a reason for hiding this comment

vdemeester left a comment

Choose a reason for hiding this comment

TheHipbot commented Feb 13, 2017

TheHipbot commented Feb 15, 2017

AkihiroSuda commented Feb 20, 2017

TheHipbot commented Feb 21, 2017

AkihiroSuda commented Feb 21, 2017

yongtang Feb 21, 2017

Choose a reason for hiding this comment

yongtang commented Feb 21, 2017 • edited

TheHipbot commented Feb 22, 2017

tonistiigi commented Mar 9, 2017

tonistiigi left a comment

Choose a reason for hiding this comment

yongtang Mar 9, 2017

Choose a reason for hiding this comment

yongtang Mar 9, 2017

Choose a reason for hiding this comment

TheHipbot Mar 13, 2017

Choose a reason for hiding this comment

TheHipbot commented Mar 13, 2017

vdemeester left a comment

Choose a reason for hiding this comment

tonistiigi commented Apr 13, 2017

thaJeztah left a comment

Choose a reason for hiding this comment

AkihiroSuda Feb 13, 2017 •

edited

yongtang commented Feb 21, 2017 •

edited